-
Notifications
You must be signed in to change notification settings - Fork 220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add automated release workflow #2586
Add automated release workflow #2586
Conversation
I'm slightly leaning towards setting the date of the release as it's easier to interpret by the one who reads it, but I'm not sure if it's an industry standard. But if we decide to release on every push, then I'd prefer to have number of commits encoded.
It feels like if we go with %commits since tag%, then releasing every push should be the way to go. Otherwise something like monthly releases looks good to me. @aratajew as IGC relies on the translator it might impact you. What would be your preference?
IMHO automatically generated changelog is OK
It's a good question. Such tags imho should be created manually, this automation should use the latest among them to create new patch/timestamp tags. |
Thanks for putting up a proposal!
In #1898 it seems the proposal was monthly. That sounds good to me as a start, if we want more (or less) frequent releases we can always tune the interval later.
Perhaps it would be best to follow semantic versioning, so we'd need to change the format slightly to comply. Or we could follow a different non-standard format to include the year+month.
We're not really following the patch releases of llvm since all 14.0.x releases should be compatible. For releases that break compatibility, we may need to decide when it happens perhaps?
Following what we have done for current releases sounds good to me. |
I think this would give a lot of "tag/release pollution", so I'd advise against that. |
Thank you for the feedback! I love the idea to follow sem version specification, but package relies on llvm versioning. Should we then keep only major version and keep two others in sync with specification, or should we keep major and minor in sync with llvm and update only the third part on our own. Speaking of, should not Since these are automated release, keeping updated one number should be easier than updating two. So it should be something either
What is the drawback? It seems like backport releases are not that frequent. since 14'th release there is less than 200 commits. What should we do if there are no updates since last release (e.g. a moth). Do clients what to have backports imminently on commit merge? Should we add option to manually release if such thing is critical? How would it fit into versioning? |
Practically speaking this might be the easiest, because in this project itself we haven't really differentiated between minor and patch versions until now. So aligning major and minor to llvm seems reasonable. We should probably document that, maybe you can add a section to README.md too?
Good point, or even
One concern is that it makes https://github.com/KhronosGroup/SPIRV-LLVM-Translator/releases harder to navigate. Right now it's an ordered list with the newest release on top. If we make an automatic release for every backport, the list will probably become large and "unordered" (it will reflect the order of branches getting a commit I suppose, which isn't very useful and might confuse folks).
Don't create a new release in that case I'd say, as it doesn't add any value.
Manual releases should always remain an option. For versioning, any new release could follow a 18.1. |
From my observation - it will be ordered semantically. The PR is made to apply latest tag only for most recent major release. Summarizing everything, thing to be updated:
@svenvh who should approve this design so I can get the PR updated and ready to merge? And what are the next steps? |
I'm fine with the schema, thanks for working on this! I'd like to also hear from @kurapov-peter as he is a potential customer of the feature. |
Thank you @svenvh , @MrSidims ! I'll try to update the PR till the EOW. Should we wait for @aratajew ? And something I've been thinking in terms of usability. Are all those backports ever introduce breaking compatibility? I'm thinking of other project using it through dynamic linking, could they pin project in the way |
No preference as long as it's clear what's latest (and now it is) |
This looks great and I read through the conversation thus far and it sounds like a nice addition. Will we be adding a README file on how to consume these packages or is that expected to be a known method? thanks @ZzEeKkAa for adding this.. |
bff56df
to
7412ac1
Compare
Thank you everybody! This PR is now ready to go. Just one open question that can be addressed in future releases (in other words, can we guarantee semantic versioning compatibility):
|
I guess it is known method so far... |
It appears that is not strictly ordered... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Can be done in a separate PR: IMHO it's a good idea to move a little bit groomed PR description into a separate doc placed here https://github.com/KhronosGroup/SPIRV-LLVM-Translator/tree/main/docs
@svenvh are you OK to merge this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one nit; it would indeed be nice to have some documentation too (adding to README.md
?).
@ZzEeKkAa works for me, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the doc update!
Thank you for reviews and merging it! Could you please go |
Sure! Lets see how it goes https://github.com/KhronosGroup/SPIRV-LLVM-Translator/actions/runs/9386710465 |
And it works, https://github.com/KhronosGroup/SPIRV-LLVM-Translator/releases/tag/v18.1.1! @ZzEeKkAa thanks a lot for your contribution! |
Similarly to KhronosGroup/SPIRV-LLVM-Translator#2586 adds automated patch release generation. Examples of releases could be found in my fork: https://github.com/ZzEeKkAa/opencl-clang/releases For the final result look at https://github.com/ZzEeKkAa/opencl-clang/releases/tag/v17.0.5 . On other releases I've been playing around with release message. Fixes #537
Some of the upstream PRs are getting backported to the
llvm_release_*
branches, but those changes are never released. That prevents them from distributing as precompiled packages in various distributions likeconda-forge
and others. This PR targets this issue by creating automated workflow that is triggered once a month and generates automated releases for each such branch if there were changes since last release.This PR
llvm_release_*
branches in the format%llvm_major%.%llvm_minor%.%latest patch version +1%
. For example:Merge process
Note
There is no need to backport changes to all branches, since workflow dispatch on schedule basis can be performed only from main branch.
Original proposal
I want to create this PR as a DRAFT and initial discussion about automated release. So I would like to hear feedback before merging the PR.
This PR:
llvm_release_*
branches in the format%llvm_version%.%commits since tag%
. For example:Merge process:
llvm_release_*
branchesOpen questions:
Fixes: #1898
Fixes: #1508